Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixing typos for include in requirements.txt in examples/http/*. #212

Closed
wants to merge 27 commits into from

Conversation

rcbjBlueMars
Copy link
Contributor

Description

Ticket #211. Reorganizing the examples/http directory broke the requirements.txt references to the top-level requirements.txt file.

Testing

Tested locally to make sure these now find the top-level requirements.txt

Checklist:

Standard test suite is being run.

Documentation

No documentation changes.

@jcchavezs
Copy link
Collaborator

Would you fix the lint @rcbjBlueMars ?

Specifying exact test and mgmt tools (pytest, tox, pylint, pdoc3) versions.
@rcbjBlueMars
Copy link
Contributor Author

rcbjBlueMars commented Jun 3, 2021

Would you fix the lint @rcbjBlueMars ?

The pylint errors are related to the upstream OTel instrumentation modules removing the modules being instrumented as dependencies. See [1]. Up till now we had been depending on that to make sure all the correct modules were picked up. I'm working on adding the modules (flask, aiohttp, mysql.connector, etc) to the relevant test directories.

I'm going to keep the OTel instrumentation modules we depend on in the setup.py dependency list. The end modules aren't actually used unless someone tries to enable the corresponding module. In theory, this wouldn't be done unless they are actually using that module.

Reference:
[1] open-telemetry/opentelemetry-python-contrib#475

@rcbjBlueMars
Copy link
Contributor Author

Closing this PR and adding changes to a new branch based on updated test suite that runs correctly.

@rcbjBlueMars rcbjBlueMars deleted the feature/211 branch June 5, 2021 08:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants